refactor(anta): Refactor VerifyInterfacesStatus test for nicer failure message #899
+63
−37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vitthalmagadum
changed the title
vitthalmagadum
force-pushed
the
issue_587_nicer_failure_msgs
branch
from
4d65a1d
to
33fea61
Compare
CodSpeed Performance ReportMerging #899 will not alter performanceComparing Summary
|
MaheshGSLAB
reviewed
Oct 28, 2024
| - If line protocol status is provided, prioritize checking against both status and line protocol status | ||
| - If line protocol status is not provided and interface status is "up", expect both status and line protocol to be "up" | ||
| - If interface status is not "up", check only the interface status without considering line protocol status | ||
| This test performs the following checks for each specified interface: |
There was a problem hiding this comment.
This format will not reflect properly in documentation. Please change it with hyphen
There was a problem hiding this comment.
You can use numbered list but you need to put an extra indent.
vitthalmagadum
force-pushed
the
issue_587_nicer_failure_msgs
branch
from
709b125
to
2f22a23
Compare
|
|
@vitthalmagadum please fix the Sonarlint issue |
Comment on lines
+14
to
+20
| if TYPE_CHECKING: | ||
| import sys | ||
|
|
||
| if sys.version_info >= (3, 11): | ||
| pass | ||
| else: | ||
| pass |
There was a problem hiding this comment.
Not required for now. Let's remove this.
| This test performs the following checks for each specified interface: | ||
|
|
||
| - If `line_protocol_status` is defined, both `status` and `line_protocol_status` are verified for the specified interface. | ||
| - If `line_protocol_status` is not provided but the `status` is "up", it is assumed that both the interface and line protocol should be "up". |
There was a problem hiding this comment.
Suggested change
| - If `line_protocol_status` is not provided but the `status` is "up", it is assumed that both the interface and line protocol should be "up". | |
| - If `line_protocol_status` is not provided but the `status` is "up", it is assumed that both the status and line protocol should be "up". |
| for interface in self.inputs.interfaces: | ||
| if (intf_status := get_value(command_output["interfaceDescriptions"], interface.name, separator="..")) is None: | ||
| intf_not_configured.append(interface.name) | ||
| self.result.is_failure(f"{interface.name} - not configured") |
There was a problem hiding this comment.
Suggested change
| self.result.is_failure(f"{interface.name} - not configured") | |
| self.result.is_failure(f"{interface.name} - Not configured") |
| @@ -261,18 +252,15 @@ def test(self) -> None: | |||
| # If line protocol status is provided, prioritize checking against both status and line protocol status | |||
| if interface.line_protocol_status: | |||
| if interface.status != status or interface.line_protocol_status != proto: | |||
| intf_wrong_state.append(f"{interface.name} is {status}/{proto}") | |||
| actual_state = f"Expected: {interface.status}/{interface.line_protocol_status}, Actual: {status}/{proto}" | |||
| self.result.is_failure(f"Interface status/Line protocol status for {interface.name} - {actual_state}") | |||
There was a problem hiding this comment.
Suggested change
| self.result.is_failure(f"Interface status/Line protocol status for {interface.name} - {actual_state}") | |
| self.result.is_failure(f"{interface.name} - {actual_state}") |
| if intf_wrong_state: | ||
| self.result.is_failure(f"The following interface(s) are not in the expected state: {intf_wrong_state}") | ||
| elif interface.status == "up" and (status != "up" or proto != "up"): | ||
| self.result.is_failure(f"Interface status/Line protocol status for {interface.name} - Expected: up/up, Actual: {status}/{proto}") |
There was a problem hiding this comment.
Suggested change
| self.result.is_failure(f"Interface status/Line protocol status for {interface.name} - Expected: up/up, Actual: {status}/{proto}") | |
| self.result.is_failure(f"{interface.name} - Expected: up/up, Actual: {status}/{proto}") |
| elif interface.status == "up" and (status != "up" or proto != "up"): | ||
| self.result.is_failure(f"Interface status/Line protocol status for {interface.name} - Expected: up/up, Actual: {status}/{proto}") | ||
| elif interface.status != status: | ||
| self.result.is_failure(f"Interface status for {interface.name} - Expected: {interface.status}, Actual: {status}") |
There was a problem hiding this comment.
Suggested change
| self.result.is_failure(f"Interface status for {interface.name} - Expected: {interface.status}, Actual: {status}") | |
| self.result.is_failure(f"{interface.name} - Expected: {interface.status}, Actual: {status}") |
| @@ -230,29 +235,15 @@ class Input(AntaTest.Input): | |||
| interfaces: list[InterfaceState] | |||
| """List of interfaces with their expected state.""" | |||
|
|
|||
There was a problem hiding this comment.
Here we need to keep a class variable to maintain the current behavior:
InterfaceState: ClassVar[type[InterfaceState]] = InterfaceState
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactor VerifyInterfacesStatus test for nicer failure message
Fixes #587
Checklist:
pre-commit run)tox -e testenv)